-
Notifications
You must be signed in to change notification settings - Fork 485
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add git info to title screen & gameplay stats #4053
Conversation
I think rather than checking if the branch starts with This would make it so that even nightly builds or builds from the develop-rando branch would display the git details too. Especially since develop still shows the version as the "latest" named release. |
I don't think that'd work as the builds get made before the commit is tagged. |
Changed it. |
CMakeLists.txt
Outdated
@@ -9,6 +9,37 @@ project(Ship VERSION 8.0.5 LANGUAGES C CXX) | |||
set(PROJECT_BUILD_NAME "MacReady Foxtrot" CACHE STRING "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i've been hoping to be able to remove the hardcoded version number and version name, maybe it'd be possible to pull these from the tag too (i've generally made sure that tags have the named version as the description)
the question then becomes what do we do for builds that aren't releases - we'll still need to set a version number, maybe for the name we could just have "MacReady" for the named release branch and then "Nightly" or "Develop" for the main dev branch
in general i'd be very happy to see the extra info make it into the builds, i just want to make sure we think through all of the use cases we want to support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the question then becomes what do we do for builds that aren't releases - we'll still need to set a version number, maybe for the name we could just have "MacReady" for the named release branch and then "Nightly" or "Develop" for the main dev branch
Do you mean for it to be just MacReady
/Develop
with no second part?
If so we would still need to have MacReady
somewhere in there as I see no way of getting it from the branch reliably.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so i'm not quite sure what we'd want to set those values to when not on a tagged version, but i'm thinking we could just not show them on the logo screen when not on a tagged version
i'm still really hoping we can figure out a way to use the git tag description to set the version name for tagged versions instead of needing to hardcode it, it seems we can get that by using
git cat-file tag <tag_version> | sed '1,/^$/d'
but in my quick poking around i wasn't able to get that working in cmake
basically i'm hoping we can do
project(Ship VERSION "${GIT_COMMIT_TAG}" LANGUAGES C CXX)
set(PROJECT_BUILD_NAME "${GIT_TAG_DESCRIPTION}" CACHE STRING "")
when we have a tag, and then maybe just do
project(Ship VERSION 0.0.0 LANGUAGES C CXX)
set(PROJECT_BUILD_NAME "Unreleased" CACHE STRING "")
when we don't have a tag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit hesitant on 0.0.0
since that version info is what gets stored into the game otrs and is used for enforcing the regenerate flow. I know what we have isn't great either since the develop branch wont complain about an otr from the current release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving as-is.
I know there was still conversation about potentially pulling out tag/release name info out of source entirely, but the original goal of this PR was to make tracing what commit a nightly build actually came from which would be good to merge in as testing for rando V3 increases. We can always open a separate issue to track the conversation about tag/release name in source.
100% agree that can be a separate PR could we make it so we only show the version number and name on tagged builds? i expect people would say "i'm on macready foxtrot develop rando" if we show both on the title screen when reporting from test builds - i think it'd be clearer to only show the branch name and commit sha instead |
I don't think that's a good idea as with the version and name there we can also quickly know if a build is severely outdated in the future. |
wouldn't the date do that? |
I forgot about the date. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The branch shows up if it the current commit doesn't have a tag (see here).
In the title screen, the commit hash will appear if the oot debug mode is enabled; in the gameplay stats, it'll appear if
Show Debug Info
is enabled.Build Artifacts